feat: add RSS read adapter - #8733
Conversation
|
A few points/issues about the current state:
|
fstagni
left a comment
There was a problem hiding this comment.
Regarding your questions: I would suggest expanding/modifying DIRAC"s RSS to match DiracX way of doing things.
There was a problem hiding this comment.
There is some code de-duplication that can be achieved in this module, at the same time maybe reducing readability. Up to you.
|
|
||
|
|
||
| def get_fts_status() -> list[tuple]: | ||
| """Get merged FTS server status from all VOs. |
| def _translate_resource_status(status: AllowedStatus | BannedStatus) -> str: | ||
| """Translate a single resource status from diracx format to legacy format. | ||
|
|
||
| Args: | ||
| status: ResourceStatus (AllowedStatus or BannedStatus) | ||
|
|
||
| Returns: | ||
| Legacy status string - only "Active" or "Banned" as per DIRAC streamlining | ||
|
|
||
| """ | ||
| if isinstance(status, AllowedStatus): | ||
| # DIRAC is being streamlined to only use Active/Banned states | ||
| # All allowed statuses (including Degraded) are mapped to Active | ||
| return "Active" | ||
| else: # BannedStatus | ||
| # All banned statuses are mapped to Banned | ||
| return "Banned" |
There was a problem hiding this comment.
@HeloiseJoffe is going to make a small utility that you will be able to reuse.
|
|
||
| from typing import Dict | ||
|
|
||
| from diracx.core.models.rss import ( |
There was a problem hiding this comment.
Is it working knowing that we get results from autorest models?
There was a problem hiding this comment.
You can't add RssReadAdapter here.
src/DIRAC only contains systems, here you have a specific RSS client so that should ideally go somewhere in /src/DIRAC/ResourceStatusSystem/Client.
In this context, I think you can get rid of this init file
There was a problem hiding this comment.
What about merging ResponseTranslation and Statuses into a single file under src/DIRAC/ResourceStatusSystem/Client?
| List of tuples in legacy format: (name, element_type, status_type, status, vo) | ||
|
|
||
| """ | ||
| with DiracXClient() as client: |
There was a problem hiding this comment.
Can you pass the client as an argument of the functions? So that a same one can be reused if needed
| ) | ||
|
|
||
|
|
||
| def get_storage_element_status() -> list[tuple]: |
There was a problem hiding this comment.
I guess you would just need to have 1 function that would have almost the same form as ResourceStatusClient.selectStatusElement.
And in the next PR that would be used in SiteStatus.__updateRssCache and ResourceStatus.__updateRssCache IIUC.
There was a problem hiding this comment.
Oh okay I understand but should I make 1 function updating all rss statuses or 1 for Resources and 1 for Sites?
There was a problem hiding this comment.
Here we have 1 for both Site and Resource. Do you think there is a good reason for having 2?
There was a problem hiding this comment.
In my opinion it doesn't make much sense to update Resources statuses in the rss cache in the SiteStatus.__updateRssCache or update Site statuses in ResourceStatus.__updateRssCache, also the expected output format is different because of the difference of meta argument. For me it's just different enough to be two different functions.
True, may be we would need to rethink the DiracX RSS API? @fstagni any opinion? |
Yes, as I wrote before we can change also DIRAC RSS API |
closes #8685
BEGINRELEASENOTES
*RssReadAdapter
NEW: add RssReadAdapter module
ENDRELEASENOTES